Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

standardize verbosity of ZenodoRecord methods #58

Merged
merged 1 commit into from
Oct 2, 2021
Merged

standardize verbosity of ZenodoRecord methods #58

merged 1 commit into from
Oct 2, 2021

Conversation

jeffreyhanson
Copy link
Contributor

@jeffreyhanson jeffreyhanson commented Oct 2, 2021

Hi @eblondel,

Thank you very much for developing this package! I was playing around with it and AFAIK it would seem that querying available versions for a DOI results in logging information being printed (regardless of the desired logging level). For example, please consider this reprex:

library(zen4R)
doi <- "10.5281/zenodo.4058356"
z <- ZenodoManager$new()
v <- z$getRecordByDOI(doi)$getVersions()
#> [zen4R][INFO] ZenodoRequest - Fetching https://zenodo.org/api/records?q=conceptrecid:3666245&size=10&page=1&all_versions=1 
#> [zen4R][INFO] ZenodoManager - Successfully fetched list of published records - page 1 
#> [zen4R][INFO] ZenodoRequest - Fetching https://zenodo.org/api/records?q=conceptrecid:3666245&size=10&page=2&all_versions=1 
#> [zen4R][INFO] ZenodoManager - Successfully fetched list of published records! 

My understanding is that this issue is caused by a new ZenodoManager object being created in the ZenodoRecord$getVersions() method with logger = "INFO"). To resolve this, I suggest (per this PR) removing the logging information and using the default ZenodoManager$new() constructor. This change would be in line with other ZenodoRecord methods that create new ZenodoManager objects (e.g. here, here, here). This issue is really minor in the grand scheme of things - I just thought it might be worth mentioning since the package in general provides excellent control over the desired logging level?

How does that sound? Please let me know if I'm missing anything? I'm sorry if the documentation details a method for silently querying available versions, and I simply haven't read the documentation closely enough?


Session information

R version 4.1.1 (2021-08-10)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 21.04

Matrix products: default
BLAS:   /usr/lib/x86_64-linux-gnu/blas/libblas.so.3.9.0
LAPACK: /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.9.0

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] zen4R_0.5      testthat_3.0.4 devtools_2.4.2 usethis_2.0.1 

loaded via a namespace (and not attached):
 [1] xml2_1.3.2        magrittr_2.0.1    pkgload_1.2.1     R6_2.5.1         
 [5] rlang_0.4.11      fastmap_1.1.0     httr_1.4.2        tools_4.1.1      
 [9] pkgbuild_1.2.0    sessioninfo_1.1.1 cli_3.0.1         withr_2.4.2      
[13] ellipsis_0.3.2    remotes_2.4.0     assertthat_0.2.1  rprojroot_2.0.2  
[17] lifecycle_1.0.0   crayon_1.4.1      keyring_1.2.0     processx_3.5.2   
[21] purrr_0.3.4       callr_3.7.0       fs_1.5.0          ps_1.6.0         
[25] curl_4.3.2        memoise_2.0.0     glue_1.4.2        cachem_1.0.6     
[29] compiler_4.1.1    desc_1.3.0        prettyunits_1.1.1 jsonlite_1.7.2 

@eblondel eblondel merged commit 969053a into eblondel:master Oct 2, 2021
@eblondel
Copy link
Owner

eblondel commented Oct 2, 2021

Thanks @jeffreyhanson , just merged!

@jeffreyhanson
Copy link
Contributor Author

Awesome - thanks!

eblondel added a commit that referenced this pull request Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants